Skip to content

[FIX] wrong handling of border.rectList with PySide6 backend #103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
PierreRaybaut merged 8 commits into from
Jul 14, 2025
Merged

[FIX] wrong handling of border.rectList with PySide6 backend #103

PierreRaybaut merged 8 commits into from
Jul 14, 2025

Conversation

@martinschwinzerl
Copy link
Contributor

@martinschwinzerl martinschwinzerl commented Jul 11, 2025

This appears to fix the problems described in issue #102 . The changes can be made more pythonic, this patch literally came out of my debugging session - feedback regarding cleaning it up and making it ready for merging would be highly appreciated

Cheers,
Martin

martinschwinzerl and others added 5 commits July 12, 2025 01:39
@martinschwinzerl
[FIX] wrong handling of border.rectList with PySide6 backend
44d9005 
This appears to fix the problems described in issue PlotPyStack#102 .
The changes can be made more pythonic, this patch literally came out
of my debugging session - feedback regarding cleaning it up and
making it ready for merging would be highly appreciated
@martinschwinzerl
[FIX] missing generator expression for all(isinstance) check
0531934
@martinschwinzerl
[FIX] missing generator expression for all(isinstance) check
ab18915
@martinschwinzerl
Merge remote-tracking branch 'origin/fix/102_test_stylesheet_fail_pys…
93b3c28 
…ide6' into fix/102_test_stylesheet_fail_pyside6
@PierreRaybaut
Update plot_canvas.py
21333e9 
Here is my proposition to keep one of the case that was implemented for issue PlotPyStack#63
@PierreRaybaut PierreRaybaut mentioned this pull request Jul 12, 2025
Closed
@martinschwinzerl
[UPDATE] fix wrong attribute name
80ec6ce 
The class Border has a rectList member, but some lines in
drawRects() refered to a member attribute named list. This caused the
test to fail again on PySide6, test passes now with these modifications.
@martinschwinzerl
Copy link
Contributor Author

martinschwinzerl commented Jul 13, 2025

Thanks for looking into this, there was still an issue, as described in the commit message. Also, shouldn't line 77 be something like

self.border.rectList.append(rects)

to be consistent across all cases?
Regardless, with 80ec6ce, the test passes on my side (PySide6 6.9.1, Linux x86_64, Python 3.12).

Cheers,
Martin

@PierreRaybaut
Copy link
Collaborator

PierreRaybaut commented Jul 13, 2025

Thanks for looking into this, there was still an issue, as described in the commit message. Also, shouldn't line 77 be something like

self.border.rectList.append(rects)

to be consistent across all cases? Regardless, with 80ec6ce, the test passes on my side (PySide6 6.9.1, Linux x86_64, Python 3.12).

Cheers, Martin

Sorry, I wrote my commit directly online without testing anything, hence the obvious mistakes. Thanks for fixing them.
I'm suggesting another solution in a minute or so, could you please test it?

@martinschwinzerl
Copy link
Contributor Author

martinschwinzerl commented Jul 14, 2025

Thanks for the update!
All tests pass with the proposed changes in bdb7e43 at least on my system.

Cheers,
Martin

@PierreRaybaut PierreRaybaut merged commit 17b18ac into PlotPyStack:master Jul 14, 2025
11 of 12 checks passed
@PierreRaybaut PierreRaybaut changed the title [FIX] wrong handling of border.rectList with PySide6 backend [FIX] wrong handling of border.rectList with PySide6 backend Jul 14, 2025
PierreRaybaut added a commit that referenced this pull request Jul 14, 2025
@PierreRaybaut
Changelog: add entry for PR #103: fix handling of border.rectList wit…
09c1d30 
…h PySide6 backend
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@martinschwinzerl @PierreRaybaut